Skip to content

🌱 cleanup: build #3280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 27, 2024
Merged

Conversation

perdasilva
Copy link
Collaborator

@perdasilva perdasilva commented May 27, 2024

Description of the change:

  • Removes old unused script files and e2e test resources
  • Substitutes Dockerfile for Dockerfile.goreleaser
  • Removes e2e.Dockerfile and (old) Dockerfile
  • Removes image GHA (we build the image for e2e anyway)
  • Adds golangci-lint to tools
  • Cleans-up Makefile and add help target

Fast follows from this:

  • Merge sanity checks and verification GHAs and drop golangci-lint action in favor of calling make lint (make sure the golangci-lint versions on dev and CI are the same

Motivation for the change:
Removing cruft from the project and trying to simplify things. Bumping k8s to 1.30 is causing some flakes. Having a tidy Makefile will make it easier to work with the codebase and fix issues.

Architectural changes:

Testing remarks:

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Bug fixes are accompanied by regression test(s)
  • e2e tests and flake fixes are accompanied evidence of flake testing, e.g. executing the test 100(0) times
  • tech debt/todo is accompanied by issue link(s) in comments in the surrounding code
  • Tests are comprehensible, e.g. Ginkgo DSL is being used appropriately
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive
  • Tests marked as [FLAKE] are truly flaky and have an issue
  • Code is properly formatted

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 27, 2024
@openshift-ci openshift-ci bot requested review from dinhxuanvu and kevinrizza May 27, 2024 06:00
@perdasilva perdasilva force-pushed the cleanup/makefile branch 3 times, most recently from 1a9b6ba to dab1ac1 Compare May 27, 2024 07:17
@perdasilva perdasilva changed the title [WIP] 🌱 cleanup: Makefile 🌱 cleanup: Makefile May 27, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 27, 2024
@@ -55,7 +55,6 @@ const (

olmConfigMap = "olm-operators" // No-longer used, how long do we keep this around?

// sync name with scripts/install_local.sh
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scripts/install_local.sh does not seem to reference anything to with the packageserver...


output:
format: tab
formats: tab
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this file are to fix warnings given by the tool

Per Goncalves da Silva added 5 commits May 27, 2024 10:52
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
@perdasilva perdasilva changed the title 🌱 cleanup: Makefile 🌱 cleanup: build May 27, 2024
Copy link
Member

@m1kola m1kola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions, but other than that - all looks good..

Refactor Makefile
clean up build targets

Signed-off-by: Per Goncalves da Silva <[email protected]>
Signed-off-by: Per Goncalves da Silva <[email protected]>
@perdasilva perdasilva enabled auto-merge May 27, 2024 11:02
Copy link
Member

@m1kola m1kola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Would still be a good to have a link to an issue here or get it reverted if it is just a warning, but it is not something which should be blocking this PR.

@m1kola
Copy link
Member

m1kola commented May 27, 2024

You will probably have to remove image job as required from GH settings.

@perdasilva perdasilva added this pull request to the merge queue May 27, 2024
Merged via the queue into operator-framework:master with commit c1b059a May 27, 2024
12 checks passed
This was referenced Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants